Skip to content

Fix passive popup/tab registration in the Node bridge server#104

Open
poman wants to merge 1 commit into
playwright-php:mainfrom
poman:fix/passive-popup-registration
Open

Fix passive popup/tab registration in the Node bridge server#104
poman wants to merge 1 commit into
playwright-php:mainfrom
poman:fix/passive-popup-registration

Conversation

@poman

@poman poman commented Jul 7, 2026

Copy link
Copy Markdown

Fixes playwright-php/playwright-mink#4

BrowserContext/the PHP client already handle a "page" event coming from the server (a new page/tab was created by the browser) and dispatch it correctly once received. The Node server, however, never subscribed to the underlying Playwright context.on('page', ...) event, so the event was never sent over the wire in the first place. Any workflow that opens a second tab/window from the page itself (window.open(), target="_blank", ...) — as opposed to a tab created through an explicit waitForPopup()/waitForEvent('page') call — was invisible to the PHP side: driver code that clicks a link and only afterwards inspects the set of open windows (the pattern Mink-based drivers use) never saw the new tab.

Adds a registerContextPopups() subscription right after each of the four places that create and register a BrowserContext (launchBrowser, newContext, connect, connectOverCDP). It forwards a "page" event with the new page's ID once it settles, deduplicated against pages already registered through the existing explicit paths (createNewPage(), waitForEvent('page'), waitForPopup()) so the same page is never registered twice.

Testing

  • Added tests/Integration/Popup/PassivePopupDiscoveryTest.php: clicks a target="_blank" link with no waitForPopup()/waitForEvent('page') armed beforehand, and asserts the new page shows up in BrowserContext::pages() — verified it fails against the pre-fix server (stays at 1 page) and passes with the fix (reaches 2).
  • Ran the full existing popup suite (tests/Integration/Popup/*, 8 tests) to confirm the passive subscription does not double-register pages already handled by waitForPopup()/waitForEvent('page')/createNewPage() — all green.
  • Full test suite: 796/796 unit, 237/237 integration passing (2 pre-existing, unrelated deprecation warnings).
  • php-cs-fixer --dry-run clean; phpstan analyse (level 10) reports 0 errors on the changed/added files; node --check on the modified server file.

Discovered while running the OroCommerce Behat regression suite on the Mink driver built on top of this library - the same root cause as playwright-php/playwright-mink#4

@codecov

codecov Bot commented Jul 7, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Windows/tabs opened e.g. by links not registered by the driver

1 participant